-
-
Notifications
You must be signed in to change notification settings - Fork 421
Add MavenPublishModule for maven compatiable repository #5601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chikei Is this code written from scratch? Is it partially copied from other places in the Mill repo? This info would greatly help reviewing it.
|
It's copied from existing Edit: and remove the gpg args from |
lefou
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, the general approach looks good.
I'd like a have the implementations of object MavenPublishModule moved into a trait and just extends that. That way, it should be easier to customize and reuse it in projects.
Also, the hard-coded use of MavenPublishModule.publishAll should instead use a ModuleRef instead.
|
Hi @lefou and @chikei , do you still consider this PR actively worked on? If not I would like to attempt this (though it does feel a bit out of my depth). @lefou regarding your last comment about (1) moving the implementations of |
It would be nice to get this over the finish line. Since the base functionality already done, left work should be mostly mechanical editing.
Sharing common parts between both traits would be the best case, of course. Whether we do it right now or in a subsequent PR is both fine for me, as long as we get it merged rather soon. |
) ### Summary Implements #5592 and builds upon #5601. ### High Level Overview of Changes This PR implements three traits: - `MavenPublishModule` is meant to mirror `SonatypeCentralPublishModule` for plain (!?) Maven repositories - `MavenModule` contains the actual publishing logic. This has been taken from parts of `SonatypeCentralPublishModule` and is now extended by both `MavenPublishModule` as well ass `SonatypeCentralPublishModule` to share code - `MavenCredentialsModule` contains tasks for obtaining Maven/Sonatype credentials. These tasks have been extracted from the `SonatypeCentralPublishModule` to be shared by both it and the new `MavenPublishModule` ### Differences between this PR and #5601 #### Code Structure #5601 essentially copied parts of `SonatypeCentralPublishModule` to create `MavenPublishModule`. Apart from duplicating code, this also created code you may find confusing in places (e.g. by containing a function named [publishSnapshot](https://github.com/com-lihaoyi/mill/pull/5601/files#diff-7ffbad225d6fa5d1de00e7ea2066f029c9fe4b90c09785ac3cd51ea0d12b3638R126) which is used to publish both release and snapshot artifacts). This PR tries to improve upon that by instead factoring out all code which can be factored out. #### Removal of Unused Settings #5601 defines the tasks `mavenConnectTimeout`, `mavenReadTimeout` and `mavenAwaitTimeout` as part of the `MavenPublishModule`. These are however not actually used anywhere. They are used in `SonatypeCentralPublishModule` and are specific to publishing releases to Sonatype Central. #### Bugfix for Choosing the Repo URI The below was changed after being identified during testing with a private build. ```diff - val uri = if (isSnapshot) releaseUri else snapshotUri + val uri = if (isSnapshot) snapshotUri else releaseUri ``` ### Open Question: Build Compilation Error When Extending `MavenPublishModule` When running `./mill dist.installLocalCache` and then using the local `SNAPSHOT` version of mill including the changes made in this PR in a build and extending `MavenPublishModule` in a local build module, I currently get the following exception when running `./mill resolve _` on that build: ``` [build.mill-59] [error] ## Exception when compiling 23 sources to /home/max/Repositories/github.com/ttmzero/rtp-backend/out/mill-build/compile.dest/classes [build.mill-59] [error] dotty.tools.dotc.core.MissingType: Cannot resolve reference to type com.lumidion.sonatype.central.client.core.type.SonatypeCredentials. [build.mill-59] [error] The classfile defining the type might be missing from the classpath. ``` This error can be removed by defining `Deps.sonatypeCentralClient` as a `mvnDeps` instead of both a `compileMvnDeps` and a `runMvnDeps` in `libs/scalalib/package.mill` and `libs/javalib/package.mill`. I do not know why this is the case and would require some help here. --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
…6231) ### Summary Implements #5592 and builds upon #5601. ### High Level Overview of Changes This PR implements three traits: - `MavenPublishModule` is meant to mirror `SonatypeCentralPublishModule` for generic Maven repositories - `MavenModule` contains the actual publishing logic. This has been taken from parts of `SonatypeCentralPublishModule` and is now extended by both `MavenPublishModule` as well ass `SonatypeCentralPublishModule` to share code - `PublishCredentialsModule` contains tasks for obtaining Maven/Sonatype credentials. These tasks have been extracted from the `SonatypeCentralPublishModule` to be shared by both it and the new `MavenPublishModule` --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
more like a POC for #5592, I think there are rooms for sharing code/task between this and SonatypeCentralPublishModule